fix(workflows): escape the literal bracket in workflow resolve layer output - #3882
fix(workflows): escape the literal bracket in workflow resolve layer output#3882jawwad-ali wants to merge 1 commit into
workflow resolve layer output#3882Conversation
`workflow_resolve` printed each layer as
f" • [{layer.tier}] {layer.source} (priority={priority})"
Rich parses `[base]` and `[project-overlay]` as style tags, so the tier
column was silently swallowed on every run:
BEFORE: '* .specify/workflows/wf/workflow.yml (priority=n/a)'
AFTER : '* [base] .specify/workflows/wf/workflow.yml (priority=n/a)'
The tier is one of the two things the command exists to report, and the
JSON payload still carried it correctly — only the human-readable output
lost it.
The existing test asserts `"base" in result.output`, which passes anyway
because the step-attribution section prints the same word as a source, so
the missing column was masked.
Escape the literal bracket with `\[`, matching presets/_commands.py:378,
and escape the user-controlled layer sources and step ids while there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes Rich rendering so workflow resolution displays layer tiers and bracket-containing identifiers correctly.
Changes:
- Escapes literal brackets and interpolated values in resolution output.
- Adds regression coverage for tier labels.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/overlays/_commands.py |
Escapes Rich markup in layer and attribution output. |
tests/workflows/test_overlay_commands.py |
Verifies tier labels remain visible. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Closing this as superseded — Main's version does everything this PR does: console.print(
f" • \[{_escape_markup(layer.tier)}] "
f"{_escape_markup(layer.source)} "
f"(priority={priority})"
)
...
console.print(
f" • {_escape_markup(composed.step_id)}: "
f"{_escape_markup(composed.source)}"
)…including the step-attribution escaping, and it ships It also answers the review comment on this PR better than I could have: its comment records that step IDs "come from base-workflow / overlay YAML, which only bans No point carrying a duplicate through review. Thanks for the look regardless — the observation that my test left the step-ID escaping unprotected was correct, and I've applied the same "does removing this line still pass?" check to the other PRs in this batch. |
Problem
workflow_resolveinsrc/specify_cli/workflows/overlays/_commands.pyprints each layer like this:f" • [{layer.tier}] {layer.source} (priority={priority})"Rich parses
[base]and[project-overlay]as style tags, so the tier column is silently swallowed — on every single run, not just for odd input.Reproduction on current
main(81bf741)Note the double space in the BEFORE lines — that is where the tier used to be. The layer's tier is one of the two things
workflow resolveexists to tell you (which layer, at what precedence), so the human-readable output is missing half its answer. The--jsonpayload carriestiercorrectly; only the printed form loses it.Why the existing test didn't catch it
test_workflow_resolveasserts:That passes with the bug, because the step-attribution section prints the same word as a step source. The tier has to be asserted in bracket form, on the layers line itself — which is what the new test does.
Fix
Escape the literal bracket with
\[, exactly aspresets/_commands.py:378already does:I also escaped the interpolated values in the same output block —
layer.sourceis a filesystem path andcomposed.step_id/composed.sourceare user-authored ids, all of which can contain brackets and would break the same lines. That keeps the whole of this one command's output correct rather than fixing half of it.No breaking change.
rich.markup.escapeis a no-op on bracket-free values, and the--jsonpayload is untouched. The only difference is that output which previously lost characters now shows them.Verification
srcand passes with the fix. File: 1 failed → 21 passed.tests/workflows: failure set identical to the clean-mainbaseline captured on81bf741(10 pre-existing in scope, all Windows symlink-privilege).uvx ruff@0.15.0 check src tests→ cleanWritten with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.